feat(angular,solid,vue): add core signal reactivity feature #6181
feat(angular,solid,vue): add core signal reactivity feature #6181riccardoperra wants to merge 12 commits intoalphafrom
Conversation
|
📝 WalkthroughWalkthroughReplaces framework-specific Angular reactivity with a generic constructReactivityFeature, moves table options into an optionsStore with reactive synchronization, updates framework adapters (Angular, Vue, Solid, React) to use the new system, removes Angular reactivity utilities/tests, and adds a Solid devtools package plus an Options panel in table-devtools. Changes
Sequence Diagram(s)mermaid UI->>Table: toggle enableRowSelection Note: colored rectangles not required for this flow. Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
f3f8c5f to
67d022a
Compare
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:sherif,t... |
❌ Failed | 2m 48s | View ↗ |
nx run-many --targets=build -p @tanstack/table-... |
✅ Succeeded | 5s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-03-07 18:18:17 UTC
5cf10b3 to
00f2180
Compare
40513ea to
b5086d5
Compare
* feat: update @tanstack/store to v9 * Update packages/table-core/src/core/table/constructTable.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: lockfile * ci: apply automated fixes * feat: add solid table devtools * feat: add table devtools options panel * feat: move reactivity feature to core * feat: refactor reactivity feature with notifier impl * feat: try to refactor vue adapter * ci: apply automated fixes * wip vue adapter * wip options as store + some fixes in solid, angular adapter. fix react adapter * fix vue adapter * fix types * fix table devtools * fix lint * fix table devtools * fix knip issues --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
976cd8f to
f4d29ce
Compare
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (11)
packages/table-devtools/src/components/FeaturesPanel.tsx (1)
193-193: Remove the now-dead row-count helpers in the same pass.This render path no longer consumes
ROW_MODEL_TO_GETTER/getRowCountForModel, so those declarations are now just stale maintenance surface. I'd drop them here to keep the panel implementation aligned with what it actually renders.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/table-devtools/src/components/FeaturesPanel.tsx` at line 193, The ROW_MODEL_TO_GETTER mapping and getRowCountForModel helper are now unused by the FeaturesPanel render path (the UI only references rowModelName), so remove those dead declarations to reduce maintenance surface; locate and delete the ROW_MODEL_TO_GETTER constant and the getRowCountForModel function (and any nearby unused imports/exports that only served them) so the panel code only contains live symbols referenced by FeaturesPanel (e.g., the rowModelName rendering logic).packages/table-devtools/src/components/OptionsPanel.tsx (2)
11-17: ConditionaluseStoremay not react to late table availability.In Solid.js, the component body runs once at creation. If
table()initially returnsundefinedand later becomes available,useStoreis never called because the ternary was already evaluated. Consider usingcreateMemoor restructuring to ensure reactivity:♻️ Suggested refactor using Show or createMemo
+import { Show, createMemo } from 'solid-js' + export function OptionsPanel() { const styles = useStyles() const { table } = useTableDevtoolsContext() - const tableInstance = table() - const tableState = tableInstance - ? useStore( - tableInstance.optionsStore, - ({ state, data, _features, _rowModels, ...options }) => options, - ) - : undefined + return ( + <div class={styles().panelScroll}> + <Show when={table()} fallback={<div>No table available</div>}> + {(tableInstance) => { + const tableState = useStore( + tableInstance().optionsStore, + ({ state, data, _features, _rowModels, ...options }) => options, + ) + return ( + <ResizableSplit + left={ + <> + <div class={styles().sectionTitle}>Options</div> + <JsonTree copyable value={tableState()} /> + </> + } + right={<></>} + /> + ) + }} + </Show> + </div> + ) +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/table-devtools/src/components/OptionsPanel.tsx` around lines 11 - 17, The current ternary that calls useStore with tableInstance (table(), tableInstance, useStore, tableInstance.optionsStore) runs only once and will miss when table() is undefined at mount and later becomes available; refactor so useStore is invoked reactively — e.g., wrap access to table() in a createMemo or guard with a Solid <Show> so that when table() becomes non-undefined you call useStore(tableInstance.optionsStore, ...) and update tableState accordingly; ensure you reference the same selector ({ state, data, _features, _rowModels, ...options }) and return undefined until the table exists to preserve types and reactivity.
19-25: RedundanttableState?.()call on line 20.The first invocation is discarded. If intended for reactivity tracking, it wouldn't work here since
getStateisn't wrapped in a reactive primitive. Simplify by removing it:🧹 Simplified getState
const getState = (): unknown => { - tableState?.() if (!tableInstance) { return undefined } return tableState?.() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/table-devtools/src/components/OptionsPanel.tsx` around lines 19 - 25, The getState function contains a redundant initial call to tableState (tableState?.()) whose return value is discarded; remove that first invocation and simplify getState to only check tableInstance and then return tableState?.() so the function either returns undefined when tableInstance is falsy or the result of tableState; if the original intent was to trigger reactivity, wrap getState in the appropriate reactive primitive instead of relying on a discarded call.packages/solid-table-devtools/vite.config.ts (1)
6-15: Consider addingserver.allowedHostsfor CodeSandbox compatibility.For consistency with other Vite configs in the repository and forward compatibility with newer Vite versions, consider adding DNS rebinding protection:
const config = defineConfig({ plugins: [solid()], + server: { allowedHosts: true }, test: { name: packageJson.name,This is optional for a package build config but ensures compatibility if the dev server is used during development. Based on learnings: "add server: { allowedHosts: true } to enable DNS rebinding protection in CodeSandbox environments when using newer Vite versions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/solid-table-devtools/vite.config.ts` around lines 6 - 15, The Vite config defined by the const config = defineConfig(...) is missing server.allowedHosts which can block CodeSandbox/dev-server DNS rebinding; update the defineConfig call to include a server property with allowedHosts: true (e.g., add server: { allowedHosts: true }) so the dev server in this package (solid-table-devtools) matches other repo configs and remains compatible with newer Vite versions and CodeSandbox environments.packages/solid-table-devtools/src/TableDevtools.tsx (1)
20-23: Consider wrappingsetTableDevtoolsTargetin an effect for reactive updates.
setTableDevtoolsTarget(props.table)is called during component execution. While Solid components run once (unlike React), ifprops.tableis intended to be reactive (e.g., swapped dynamically), this won't re-execute when the table changes.♻️ Suggested refactor using createEffect
+import { createEffect } from 'solid-js' import { createSolidPanel } from '@tanstack/devtools-utils/solid' import { TableDevtoolsCore, setTableDevtoolsTarget, } from '@tanstack/table-devtools'function TableDevtoolsPanel(props: TableDevtoolsSolidInit) { - setTableDevtoolsTarget(props.table) + createEffect(() => { + setTableDevtoolsTarget(props.table) + }) return <TableDevtoolsPanelBase theme={props.theme} /> }If the table instance is expected to remain static for the lifetime of the component, the current implementation is fine. Please confirm the intended usage pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/solid-table-devtools/src/TableDevtools.tsx` around lines 20 - 23, The call to setTableDevtoolsTarget inside TableDevtoolsPanel should be moved into a Solid reactive effect so updates to props.table propagate; in TableDevtoolsPanel wrap setTableDevtoolsTarget(props.table) in createEffect (import createEffect from 'solid-js') so when props.table changes the target is updated, leaving the component to return <TableDevtoolsPanelBase theme={props.theme} /> as before; keep the original static-path behavior if you intend the table to never change, otherwise use createEffect around the setTableDevtoolsTarget call.packages/react-table/src/useTable.ts (1)
21-22: UnuseduseIsomorphicLayoutEffectdefinition.
useIsomorphicLayoutEffectis defined but never used in this file. The previous usage appears to have been removed as part of this refactor. Consider removing this dead code.🧹 Proposed fix to remove unused code
-const useIsomorphicLayoutEffect = - typeof window !== 'undefined' ? useLayoutEffect : useEffectAnd update the import:
-import { useEffect, useLayoutEffect, useMemo, useState } from 'react' +import { useEffect, useMemo, useState } from 'react'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-table/src/useTable.ts` around lines 21 - 22, Remove the unused constant useIsomorphicLayoutEffect and any now-unnecessary imports: delete the line defining useIsomorphicLayoutEffect (the conditional assignment using useLayoutEffect and useEffect) and then remove useLayoutEffect and/or useEffect from the import list in this module if they are no longer used elsewhere in useTable.ts; ensure no other code references useIsomorphicLayoutEffect after the removal.packages/table-core/src/core/table/constructTable.ts (2)
63-69: Verify the double.stateaccess is intentional.The expression
table.optionsStore.state.stateaccesses:
optionsStore.state→ the storedTableOptions.state→ theoptions.stateproperty (user-provided partial state overrides)This appears correct for overlaying user-provided state onto the base store state, but the double
.statecould be confusing. Consider adding a clarifying comment.table.store = createStore(() => { const state = table.baseStore.state return { ...state, + // Overlay user-provided state from options.state onto the base state ...(table.optionsStore.state.state ?? {}), } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/table-core/src/core/table/constructTable.ts` around lines 63 - 69, The double .state access in constructTable when assigning table.store (inside the createStore callback) is intentional: it overlays the user-provided partial state found at table.optionsStore.state.state onto the base state; add a brief clarifying comment above that line explaining that table.optionsStore.state is the TableOptions object and its .state property contains user overrides, e.g., “// overlay user-provided partial state from optionsStore.state.state onto base state”, so future readers of createStore / table.store and references to table.optionsStore.state.state are not confused.
37-42: Consider aligning the setter behavior withsetOptions.The
optionssetter directly replaces the entire state, whiletable.setOptionsperforms merging viatable_mergeOptions. This inconsistency could lead to unexpected behavior if someone usestable.options = newOptionsexpecting the same merge semantics assetOptions.Consider either:
- Making the setter call
setOptionsinternally for consistency- Documenting this as a "replace-all" operation distinct from
setOptions♻️ Option 1: Align setter with setOptions merge behavior
get options() { return this.optionsStore.state }, set options(value) { - this.optionsStore.setState(() => value) + this.setOptions(() => value) },Note: This requires
setOptionsto be available, which it isn't at construction time. An alternative is to simply remove the setter to enforce usingsetOptions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/table-core/src/core/table/constructTable.ts` around lines 37 - 42, The options setter currently replaces state via this.optionsStore.setState and diverges from table.setOptions which merges using table_mergeOptions; remove the options setter (leave only the getter) so callers must use setOptions (which uses table_mergeOptions), or alternatively replace the setter body to delegate to this.setOptions(value) if you can guarantee setOptions is available; update references to the options setter accordingly to enforce consistent merge semantics.packages/angular-table/src/injectTable.ts (1)
119-124: Commented-outoptionsNotifierbinding.The
optionsNotifieris commented out. If this is intentional (options reactivity handled differently in Angular), consider removing the comment to avoid confusion. If it's meant to be implemented, this should be tracked.const angularReactivityFeature = constructReactivityFeature({ - // optionsNotifier: () => stateNotifier(), stateNotifier: () => { return stateNotifier() }, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/angular-table/src/injectTable.ts` around lines 119 - 124, The commented-out optionsNotifier line in the constructReactivityFeature call (the optionsNotifier property next to stateNotifier inside the angularReactivityFeature instantiation) is ambiguous; either remove the commented line to avoid confusion or implement/enable it if options reactivity is required. Update the constructReactivityFeature call by either deleting the commented-out "optionsNotifier: () => stateNotifier()," line or by providing a proper optionsNotifier implementation (e.g., optionsNotifier: () => optionsNotifier() or wiring to the correct notifier) alongside the existing stateNotifier to ensure options reactivity is explicit.packages/table-core/src/features/table-reactivity/tableReactivityFeature.ts (1)
5-8: Empty interface flagged by static analysis.The
TableReactivityFeatureConstructorsinterface is empty, which Biome flags as equivalent to{}. If this is a placeholder for future extension, consider adding a comment. Otherwise, a type alias would be cleaner.♻️ Options to address empty interface
Option 1 - Add explanatory comment:
+// Placeholder for future constructors - intentionally empty for now interface TableReactivityFeatureConstructors< TFeatures extends TableFeatures, TData extends RowData, > {}Option 2 - Use type alias:
-interface TableReactivityFeatureConstructors< - TFeatures extends TableFeatures, - TData extends RowData, -> {} +type TableReactivityFeatureConstructors< + TFeatures extends TableFeatures, + TData extends RowData, +> = Record<string, never>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/table-core/src/features/table-reactivity/tableReactivityFeature.ts` around lines 5 - 8, The interface TableReactivityFeatureConstructors is empty (equivalent to {}), so replace it with a clearer construct: either add an explanatory comment above TableReactivityFeatureConstructors stating it’s intentionally empty as a future extension point, or convert it to a type alias (e.g. type TableReactivityFeatureConstructors<TFeatures extends TableFeatures, TData extends RowData> = {}) to avoid the empty-interface lint; update any references to the symbol accordingly and ensure generics (TFeatures, TData) remain declared on the alias if you choose that option.packages/solid-table/src/createTable.ts (1)
70-74: Type assertion toanyloses type safety.The
as anycast onmergedOptionsbypasses TypeScript's type checking. Consider using a more specific type assertion or restructuring to maintain type safety.- const mergedOptions = mergeProps(tableOptions, { - _features: mergeProps(tableOptions._features, { - solidReactivityFeature, - }), - }) as any + const mergedOptions = mergeProps(tableOptions, { + _features: mergeProps(tableOptions._features ?? {}, { + solidReactivityFeature, + }), + }) as TableOptions<TFeatures, TData>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/solid-table/src/createTable.ts` around lines 70 - 74, The current `mergedOptions` uses `as any`, losing type safety—replace the `as any` cast by preserving/declaring the concrete type: either call `mergeProps` with appropriate generics or explicitly type `mergedOptions` to the correct options interface (e.g., the TableOptions type used by `createTable`) and ensure the `_features` shape includes `solidReactivityFeature`; update the `mergeProps(tableOptions, {...})` invocation to return the typed result (or wrap the inner merge `mergeProps(tableOptions._features, {...})` with the correct feature type) so `mergedOptions` has a precise inferred type instead of `any`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/react/sub-components/src/main.tsx`:
- Around line 106-107: The render callback passed to table.Subscribe
intentionally ignores the selected state; update the render parameter from an
empty parameter list to an explicitly unused one by changing the anonymous
function signature used in the Subscribe child from () => ( to (_state) => ( so
the unused selector output is clearly marked; this touches the table.Subscribe
component's selector prop and its render function.
In `@examples/solid/row-selection/src/App.tsx`:
- Around line 1-6: Reorder the import block so third-party/external packages
(e.g., the `@tanstack/solid-devtools` and `@tanstack/solid-table-devtools` imports
and other external `@tanstack/`* imports) appear before local/module-relative
imports (./makeData and ./index.css) to satisfy import/order; specifically move
the `@tanstack/solid-devtools` and `@tanstack/solid-table-devtools` lines into the
external-import group above the ./makeData and ./index.css imports so the import
groups are external first, then local.
In `@examples/vue/row-selection/src/App.vue`:
- Around line 163-165: The button label is inverted: change the template
expression in the button that references enableRowSelection (used with
toggleRowSelection) so it shows 'Disable' when enableRowSelection is true and
'Enable' when false; update the interpolation for enableRowSelection accordingly
so the text reflects the action that will occur when toggleRowSelection is
clicked.
In `@packages/angular-table/src/injectTable.ts`:
- Line 114: Remove the unused leftover variable declaration `const count = 0`
from injectTable.ts; if `count` was intended for logic in functions such as
injectTable or surrounding scope, replace its usage appropriately, otherwise
simply delete the `count` declaration to eliminate the unused variable warning.
In `@packages/angular-table/tests/angularReactivityFeature.test.ts`:
- Line 182: The test is not asserting that the memoized computed path remains
stable after state transitions; add assertions for cellGetValueMemoizedCaptor
after each state change (selection, row recreation, visibility toggles) to
ensure computed(() => cell().getValue()) does not retrigger; locate where
cellGetValueMemoizedCaptor is defined and where state transitions occur (the
uses of computed(() => cell().getValue()), row selection/recreation calls, and
visibility updates) and invoke
expect(cellGetValueMemoizedCaptor).toHaveBeenCalledTimes(...) or
toHaveBeenCalledBefore/After as appropriate immediately after each transition to
verify the memoized path stayed memoized. Ensure similar assertions are added
for the other captors checked in the same test sections.
- Line 47: There are multiple test suites accidentally left skipped (e.g.,
describe.skip('Table property reactivity')) which removes Angular-side
regression coverage; remove the `.skip` from the describe.skip calls (search for
"describe.skip('Table property reactivity')" and other describe.skip occurrences
in angularReactivityFeature.test.ts), run the tests, and for any failures update
the assertions to match the new core contract (adjust expectation values or
port/replace assertions inside the affected suites) so the reactivity suites run
and pass rather than remaining disabled.
In `@packages/react-table/src/useTable.ts`:
- Around line 106-112: Replace the useEffect call that syncs options with
useIsomorphicLayoutEffect so options are applied before paint: locate the effect
currently using useEffect that calls table.setOptions((prev) => ({ ...prev,
...tableOptions })) and change it to useIsomorphicLayoutEffect, keeping the same
dependency array ([table, tableOptions]) and body to ensure tableOptions are
merged into the table store synchronously before paint.
In `@packages/solid-table-devtools/package.json`:
- Around line 7-11: The repository.directory field in the package.json is
incorrect (points to "packages/react-table-devtools"); update the repository
object so the "directory" value correctly references
"packages/solid-table-devtools" (i.e., change the repository.directory string)
and verify the repository.url remains accurate; modify the repository object in
this package.json to reflect the correct package directory.
In `@packages/solid-table-devtools/src/plugin.tsx`:
- Line 1: The file imports createSolidPlugin but never uses it; remove the
unused import statement for createSolidPlugin from the top of the module (delete
the `import { createSolidPlugin } from '@tanstack/devtools-utils/solid'` line)
so the unused symbol is eliminated and no linter/compile warning remains.
In `@packages/solid-table/src/createTable.ts`:
- Around line 60-68: The optionsNotifier signal is never triggered—call
setOptionsNotifier() whenever options change (e.g., alongside setNotifier()
inside the computed block that updates when state/options change) so
constructReactivityFeature receives option-change notifications; alternatively,
if only one notifier is intended, remove optionsNotifier and setOptionsNotifier
and stop passing optionsNotifier to constructReactivityFeature (update the
constructReactivityFeature call and related types/usages accordingly).
In `@packages/solid-table/src/createTableHelper.ts`:
- Line 16: createTableHelper's generic TData is not inferable because the
parameter tableHelperOptions is typed only with TFeatures; update the function
signature so TData appears in the parameter types (e.g., change
TableHelperOptions to accept TData as a second generic and make the parameter
type TableHelperOptions<TFeatures, TData>), or introduce a type-level inference
test and overload that forces TData into the parameter type; update the
createTableHelper<TData extends RowData, TFeatures...>(tableHelperOptions:
TableHelperOptions<TFeatures, TData>) signature (and related type
aliases/imports) and remove the now-unnecessary "as any" casts in
columnHelper/createTable usages so TypeScript can infer row shape correctly.
In `@packages/table-devtools/src/TableContextProvider.tsx`:
- Line 13: The import currently brings in stockFeatures as a type-only import;
change it to import the correct type name StockFeatures and use that in the
Table generic (replace stockFeatures with StockFeatures in the import and update
AnyTable = Table<StockFeatures, RowData>), so import type { RowData, Table,
StockFeatures } from '@tanstack/table-core' and update references (e.g.,
AnyTable) to use StockFeatures; alternatively, if you intended to use the
runtime const, remove the type-only modifier and import stockFeatures as a
value.
In `@packages/vue-table/src/createTableHelper.ts`:
- Line 15: The generic TData on createTableHelper currently lacks a default
which prevents inference; change the declaration from "TData extends RowData" to
"TData extends RowData = any" and make the tableHelperOptions parameter type
reference TData (so the options signature ties to the TData generic) so callers
don't need to supply explicit type args; update the related type use of
TableHelper_Core<TFeatures> & { ... } to preserve existing members (including
tableCreator) while allowing TData to default and be inferred via
tableHelperOptions.
In `@packages/vue-table/src/useTable.ts`:
- Around line 123-127: The current watchEffect creates a self-dependency because
notifier.value is read before being incremented; replace the watchEffect with a
watch that explicitly watches [allState, allOptions] as sources and increments
notifier.value inside the callback (optionally with { immediate: true } if the
initial run is required). Update the reactive watcher around watchEffect ->
watch so the effect does not depend on notifier and only runs when allState or
allOptions change.
---
Nitpick comments:
In `@packages/angular-table/src/injectTable.ts`:
- Around line 119-124: The commented-out optionsNotifier line in the
constructReactivityFeature call (the optionsNotifier property next to
stateNotifier inside the angularReactivityFeature instantiation) is ambiguous;
either remove the commented line to avoid confusion or implement/enable it if
options reactivity is required. Update the constructReactivityFeature call by
either deleting the commented-out "optionsNotifier: () => stateNotifier()," line
or by providing a proper optionsNotifier implementation (e.g., optionsNotifier:
() => optionsNotifier() or wiring to the correct notifier) alongside the
existing stateNotifier to ensure options reactivity is explicit.
In `@packages/react-table/src/useTable.ts`:
- Around line 21-22: Remove the unused constant useIsomorphicLayoutEffect and
any now-unnecessary imports: delete the line defining useIsomorphicLayoutEffect
(the conditional assignment using useLayoutEffect and useEffect) and then remove
useLayoutEffect and/or useEffect from the import list in this module if they are
no longer used elsewhere in useTable.ts; ensure no other code references
useIsomorphicLayoutEffect after the removal.
In `@packages/solid-table-devtools/src/TableDevtools.tsx`:
- Around line 20-23: The call to setTableDevtoolsTarget inside
TableDevtoolsPanel should be moved into a Solid reactive effect so updates to
props.table propagate; in TableDevtoolsPanel wrap
setTableDevtoolsTarget(props.table) in createEffect (import createEffect from
'solid-js') so when props.table changes the target is updated, leaving the
component to return <TableDevtoolsPanelBase theme={props.theme} /> as before;
keep the original static-path behavior if you intend the table to never change,
otherwise use createEffect around the setTableDevtoolsTarget call.
In `@packages/solid-table-devtools/vite.config.ts`:
- Around line 6-15: The Vite config defined by the const config =
defineConfig(...) is missing server.allowedHosts which can block
CodeSandbox/dev-server DNS rebinding; update the defineConfig call to include a
server property with allowedHosts: true (e.g., add server: { allowedHosts: true
}) so the dev server in this package (solid-table-devtools) matches other repo
configs and remains compatible with newer Vite versions and CodeSandbox
environments.
In `@packages/solid-table/src/createTable.ts`:
- Around line 70-74: The current `mergedOptions` uses `as any`, losing type
safety—replace the `as any` cast by preserving/declaring the concrete type:
either call `mergeProps` with appropriate generics or explicitly type
`mergedOptions` to the correct options interface (e.g., the TableOptions type
used by `createTable`) and ensure the `_features` shape includes
`solidReactivityFeature`; update the `mergeProps(tableOptions, {...})`
invocation to return the typed result (or wrap the inner merge
`mergeProps(tableOptions._features, {...})` with the correct feature type) so
`mergedOptions` has a precise inferred type instead of `any`.
In `@packages/table-core/src/core/table/constructTable.ts`:
- Around line 63-69: The double .state access in constructTable when assigning
table.store (inside the createStore callback) is intentional: it overlays the
user-provided partial state found at table.optionsStore.state.state onto the
base state; add a brief clarifying comment above that line explaining that
table.optionsStore.state is the TableOptions object and its .state property
contains user overrides, e.g., “// overlay user-provided partial state from
optionsStore.state.state onto base state”, so future readers of createStore /
table.store and references to table.optionsStore.state.state are not confused.
- Around line 37-42: The options setter currently replaces state via
this.optionsStore.setState and diverges from table.setOptions which merges using
table_mergeOptions; remove the options setter (leave only the getter) so callers
must use setOptions (which uses table_mergeOptions), or alternatively replace
the setter body to delegate to this.setOptions(value) if you can guarantee
setOptions is available; update references to the options setter accordingly to
enforce consistent merge semantics.
In `@packages/table-core/src/features/table-reactivity/tableReactivityFeature.ts`:
- Around line 5-8: The interface TableReactivityFeatureConstructors is empty
(equivalent to {}), so replace it with a clearer construct: either add an
explanatory comment above TableReactivityFeatureConstructors stating it’s
intentionally empty as a future extension point, or convert it to a type alias
(e.g. type TableReactivityFeatureConstructors<TFeatures extends TableFeatures,
TData extends RowData> = {}) to avoid the empty-interface lint; update any
references to the symbol accordingly and ensure generics (TFeatures, TData)
remain declared on the alias if you choose that option.
In `@packages/table-devtools/src/components/FeaturesPanel.tsx`:
- Line 193: The ROW_MODEL_TO_GETTER mapping and getRowCountForModel helper are
now unused by the FeaturesPanel render path (the UI only references
rowModelName), so remove those dead declarations to reduce maintenance surface;
locate and delete the ROW_MODEL_TO_GETTER constant and the getRowCountForModel
function (and any nearby unused imports/exports that only served them) so the
panel code only contains live symbols referenced by FeaturesPanel (e.g., the
rowModelName rendering logic).
In `@packages/table-devtools/src/components/OptionsPanel.tsx`:
- Around line 11-17: The current ternary that calls useStore with tableInstance
(table(), tableInstance, useStore, tableInstance.optionsStore) runs only once
and will miss when table() is undefined at mount and later becomes available;
refactor so useStore is invoked reactively — e.g., wrap access to table() in a
createMemo or guard with a Solid <Show> so that when table() becomes
non-undefined you call useStore(tableInstance.optionsStore, ...) and update
tableState accordingly; ensure you reference the same selector ({ state, data,
_features, _rowModels, ...options }) and return undefined until the table exists
to preserve types and reactivity.
- Around line 19-25: The getState function contains a redundant initial call to
tableState (tableState?.()) whose return value is discarded; remove that first
invocation and simplify getState to only check tableInstance and then return
tableState?.() so the function either returns undefined when tableInstance is
falsy or the result of tableState; if the original intent was to trigger
reactivity, wrap getState in the appropriate reactive primitive instead of
relying on a discarded call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ff29e345-df77-4b7b-b504-8e14ce2e2fdf
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (55)
examples/angular/row-selection/src/app/app.htmlexamples/angular/row-selection/src/app/app.tsexamples/angular/row-selection/src/app/selection-column/selection-column.tsexamples/react/basic-external-state/package.jsonexamples/react/basic-external-store/package.jsonexamples/react/composable-tables/package.jsonexamples/react/pagination/package.jsonexamples/react/row-selection/package.jsonexamples/react/sub-components/src/main.tsxexamples/react/with-tanstack-query/package.jsonexamples/solid/row-selection/package.jsonexamples/solid/row-selection/src/App.tsxexamples/vue/row-selection/src/App.vueknip.jsonpackages/angular-table/package.jsonpackages/angular-table/src/angularReactivityFeature.tspackages/angular-table/src/index.tspackages/angular-table/src/injectTable.tspackages/angular-table/src/reactivityUtils.tspackages/angular-table/tests/angularReactivityFeature.test.tspackages/angular-table/tests/flex-render/flex-render-table.test.tspackages/angular-table/tests/injectTable.test.tspackages/angular-table/tests/reactivityUtils.test.tspackages/react-table/package.jsonpackages/react-table/src/useTable.tspackages/solid-table-devtools/eslint.config.jspackages/solid-table-devtools/package.jsonpackages/solid-table-devtools/src/TableDevtools.tsxpackages/solid-table-devtools/src/index.tspackages/solid-table-devtools/src/plugin.tsxpackages/solid-table-devtools/src/production.tspackages/solid-table-devtools/src/production/TableDevtools.tsxpackages/solid-table-devtools/src/production/plugin.tsxpackages/solid-table-devtools/tsconfig.build.jsonpackages/solid-table-devtools/tsconfig.jsonpackages/solid-table-devtools/vite.config.tspackages/solid-table/package.jsonpackages/solid-table/src/createTable.tspackages/solid-table/src/createTableHelper.tspackages/table-core/package.jsonpackages/table-core/src/core/table/constructTable.tspackages/table-core/src/core/table/coreTablesFeature.types.tspackages/table-core/src/core/table/coreTablesFeature.utils.tspackages/table-core/src/features/table-reactivity/tableReactivityFeature.tspackages/table-core/src/index.tspackages/table-core/src/utils.tspackages/table-devtools/package.jsonpackages/table-devtools/src/TableContextProvider.tsxpackages/table-devtools/src/components/FeaturesPanel.tsxpackages/table-devtools/src/components/OptionsPanel.tsxpackages/table-devtools/src/components/Shell.tsxpackages/vue-table/package.jsonpackages/vue-table/src/createTableHelper.tspackages/vue-table/src/useTable.tspnpm-workspace.yaml
💤 Files with no reviewable changes (4)
- packages/angular-table/src/angularReactivityFeature.ts
- packages/angular-table/tests/reactivityUtils.test.ts
- packages/angular-table/src/index.ts
- packages/angular-table/src/reactivityUtils.ts
| <table.Subscribe selector={(state) => state}> | ||
| {() => ( |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check how other examples/tests use table.Subscribe to verify the expected pattern.
# Search for Subscribe usage patterns in react-table packages and examples
rg -n -A5 '<table\.Subscribe' --type tsx --type ts --type jsxRepository: TanStack/table
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Fallback search without type filters
rg -n -A5 'table\.Subscribe' -g '*.tsx' -g '*.ts'Repository: TanStack/table
Length of output: 50372
🏁 Script executed:
# Verify the specific file context to see the full Subscribe usage pattern
head -n 115 examples/react/sub-components/src/main.tsx | tail -n 15Repository: TanStack/table
Length of output: 462
Use idiomatic pattern for intentionally unused Subscribe render parameter.
The render function receives the selected state from selector but intentionally ignores it. Follow the idiomatic JavaScript pattern by prefixing unused parameters with underscore: change {() => ( to {(_state) => ( on line 107.
This makes intent explicit and aligns with the patterns used elsewhere in the codebase (e.g., examples/react/sorting/src/main.tsx).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/react/sub-components/src/main.tsx` around lines 106 - 107, The
render callback passed to table.Subscribe intentionally ignores the selected
state; update the render parameter from an empty parameter list to an explicitly
unused one by changing the anonymous function signature used in the Subscribe
child from () => ( to (_state) => ( so the unused selector output is clearly
marked; this touches the table.Subscribe component's selector prop and its
render function.
| TFeatures extends TableFeatures, | ||
| TData extends RowData = any, | ||
| > = Omit<TableHelper_Core<TFeatures, TData>, 'tableCreator'> & { | ||
| > = Omit<TableHelper_Core<TFeatures>, 'tableCreator'> & { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether Solid call sites rely on inferred row types or legacy phantom TData hints.
echo "== createTableHelper call sites =="
rg -nP -C2 --type=ts --type=tsx '\bcreateTableHelper\s*(<[^>]+>)?\s*\('
echo
echo "== legacy TData phantom hints =="
rg -nP -C2 --type=ts --type=tsx '\bTData\s*:'
echo
echo "== helper API usages that depend on row-shape inference =="
rg -nP -C2 --type=ts --type=tsx '\bcolumnHelper\.accessor\(|\bcreateTable\('Repository: TanStack/table
Length of output: 287
🏁 Script executed:
#!/bin/bash
# First, let's see the actual file being reviewed
echo "=== createTableHelper.ts content ==="
cat -n packages/solid-table/src/createTableHelper.ts
echo
echo "=== Search for createTableHelper call sites in solid-table ==="
rg -nP -C2 'createTableHelper\s*(<[^>]+>)?\s*\(' packages/solid-table/ --type ts
echo
echo "=== Search for createTableHelper usage in examples/tests ==="
rg -nP -C2 'createTableHelper\s*(<[^>]+>)?\s*\(' . --type ts | head -50Repository: TanStack/table
Length of output: 5338
🏁 Script executed:
#!/bin/bash
echo "=== TableHelperOptions definition in table-core ==="
rg -nP -A20 'type TableHelperOptions' packages/table-core/src --type ts
echo
echo "=== Check table-core createTableHelper or constructTableHelper ==="
rg -nP -A10 'export.*constructTableHelper|export.*createTableHelper' packages/table-core/src --type ts | head -60
echo
echo "=== Check if TData appears in TableHelperOptions ==="
rg -nP 'TableHelperOptions.*TData|TData.*TableHelperOptions' packages/table-core/src --type tsRepository: TanStack/table
Length of output: 2820
🏁 Script executed:
#!/bin/bash
echo "=== Check example usage of createTableHelper in solid/basic-table-helper ==="
cat -n examples/solid/basic-table-helper/src/App.tsx | head -100
echo
echo "=== Check if example uses explicit TData typing ==="
rg -B5 -A10 'createTableHelper' examples/solid/basic-table-helper/src/App.tsx
echo
echo "=== Check columnHelper usage in the example ==="
rg -B2 -A5 'columnHelper\.' examples/solid/basic-table-helper/src/App.tsx | head -40Repository: TanStack/table
Length of output: 6543
🏁 Script executed:
#!/bin/bash
echo "=== Verify that TData property is not actually declared on TableHelperOptions ==="
rg -nP 'type TableHelperOptions|interface TableHelperOptions' packages/table-core/src --type ts -A15
echo
echo "=== Check if other adapter packages (vue, svelte) have the same issue ==="
rg -nP 'tableHelperOptions: TableHelperOptions' packages --type ts | grep -v node_modulesRepository: TanStack/table
Length of output: 1899
🏁 Script executed:
#!/bin/bash
echo "=== Check svelte-table createTableHelper implementation ==="
cat -n packages/svelte-table/src/createTableHelper.ts | head -35
echo
echo "=== Verify table-core TableHelperOptions accepts only one type parameter ==="
rg -nP 'export type TableHelperOptions' packages/table-core/src/helpers/tableHelper.ts -A8Repository: TanStack/table
Length of output: 1673
🏁 Script executed:
#!/bin/bash
echo "=== Check if svelte TableHelperOptions with TData parameter is actually valid ==="
cat -n packages/svelte-table/src/createTableHelper.ts
echo
echo "=== Check table-core constructTableHelper signature again ==="
cat -n packages/table-core/src/helpers/tableHelper.ts | sed -n '44,60p'
echo
echo "=== Check if TableHelper_Core in svelte has TData parameter ==="
rg -nP 'export type TableHelper_Core|interface TableHelper_Core' packages/table-core/src --type ts -A15 | head -30Repository: TanStack/table
Length of output: 4295
🏁 Script executed:
#!/bin/bash
echo "=== Verify svelte's TableHelperOptions<TFeatures, TData> causes a type error ==="
# Check if svelte code actually compiles
cd packages/svelte-table && npx tsc --noEmit 2>&1 | grep -A3 "createTableHelper" || echo "No errors found in svelte"Repository: TanStack/table
Length of output: 723
🏁 Script executed:
#!/bin/bash
echo "=== Count type parameters in TableHelperOptions definition ==="
rg -nP 'export type TableHelperOptions' packages/table-core/src/helpers/tableHelper.ts
echo
echo "=== Check svelte's usage ==="
rg -nP 'TableHelperOptions<TFeatures, TData>' packages/svelte-table/src/
echo
echo "=== Check solid's usage ==="
rg -nP 'TableHelperOptions<TFeatures' packages/solid-table/src/
echo
echo "=== Check vue's usage ==="
rg -nP 'TableHelperOptions<TFeatures' packages/vue-table/src/Repository: TanStack/table
Length of output: 593
TData is no longer inferable from createTableHelper.
The createTableHelper function declares TData extends RowData = any as a generic parameter (line 26–28), but TData never appears in any parameter type signature. The tableHelperOptions parameter is typed as TableHelperOptions<TFeatures>, which only accepts TFeatures (table-core's definition does not support a second TData parameter). As a result, TData cannot be inferred from the call arguments and defaults to any.
This regression widens the inferred types of columnHelper and createTable, and the as any casts at lines 33 and 53 suppress TypeScript errors instead of surfacing the issue to consumers.
To restore row-shape inference, either add TData to the parameter type signature or introduce a type-level test that verifies TData inference works for callers without requiring explicit type arguments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/solid-table/src/createTableHelper.ts` at line 16,
createTableHelper's generic TData is not inferable because the parameter
tableHelperOptions is typed only with TFeatures; update the function signature
so TData appears in the parameter types (e.g., change TableHelperOptions to
accept TData as a second generic and make the parameter type
TableHelperOptions<TFeatures, TData>), or introduce a type-level inference test
and overload that forces TData into the parameter type; update the
createTableHelper<TData extends RowData, TFeatures...>(tableHelperOptions:
TableHelperOptions<TFeatures, TData>) signature (and related type
aliases/imports) and remove the now-unnecessary "as any" casts in
columnHelper/createTable usages so TypeScript can infer row shape correctly.
|
|
||
| import type { Accessor, ParentComponent, Setter } from 'solid-js' | ||
| import type { RowData, Table, TableFeatures } from '@tanstack/table-core' | ||
| import type { RowData, Table, stockFeatures } from '@tanstack/table-core' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how stockFeatures is exported from table-core
ast-grep --pattern 'export const stockFeatures = $_'
# Also check for named exports
rg -n 'stockFeatures' --type ts -g '*/table-core/src/**' -A 2Repository: TanStack/table
Length of output: 740
🏁 Script executed:
# Check the actual file to see the exact import and usage
cat -n packages/table-devtools/src/TableContextProvider.tsx | head -20
# Also check what types are available from table-core
rg -n 'type StockFeatures|export type StockFeatures' packages/table-core/src/Repository: TanStack/table
Length of output: 785
🏁 Script executed:
# Check what's available to import from table-core
rg -n 'export.*StockFeatures|export type StockFeatures' packages/table-core/src/ -A 1Repository: TanStack/table
Length of output: 430
Import StockFeatures type instead of stockFeatures value.
Line 13 incorrectly imports stockFeatures as a type-only import (import type { ... stockFeatures }), but stockFeatures is a runtime const value, not a type. Either:
-
Recommended: Use the type interface instead:
import type { RowData, Table, StockFeatures } from '@tanstack/table-core' // Then on line 16: type AnyTable = Table<StockFeatures, RowData>
-
Alternative: Remove
typefrom the import:import { stockFeatures } from '@tanstack/table-core'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/table-devtools/src/TableContextProvider.tsx` at line 13, The import
currently brings in stockFeatures as a type-only import; change it to import the
correct type name StockFeatures and use that in the Table generic (replace
stockFeatures with StockFeatures in the import and update AnyTable =
Table<StockFeatures, RowData>), so import type { RowData, Table, StockFeatures }
from '@tanstack/table-core' and update references (e.g., AnyTable) to use
StockFeatures; alternatively, if you intended to use the runtime const, remove
the type-only modifier and import stockFeatures as a value.
| TFeatures extends TableFeatures, | ||
| TData extends RowData = any, | ||
| > = Omit<TableHelper_Core<TFeatures, TData>, 'tableCreator'> & { | ||
| > = Omit<TableHelper_Core<TFeatures>, 'tableCreator'> & { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether Vue call sites depend on inferred row types or legacy phantom TData hints.
echo "== createTableHelper call sites =="
rg -nP -C2 --type=ts --type=tsx --type=vue '\bcreateTableHelper\s*(<[^>]+>)?\s*\('
echo
echo "== legacy TData phantom hints =="
rg -nP -C2 --type=ts --type=tsx --type=vue '\bTData\s*:'
echo
echo "== helper API usages that depend on row-shape inference =="
rg -nP -C2 --type=ts --type=tsx --type=vue '\bcolumnHelper\.accessor\(|\buseTable\('Repository: TanStack/table
Length of output: 287
🏁 Script executed:
# First, examine the createTableHelper.ts file to understand the current signature
head -50 packages/vue-table/src/createTableHelper.tsRepository: TanStack/table
Length of output: 1513
🏁 Script executed:
# Search for createTableHelper calls without the tsx type restriction
rg 'createTableHelper\s*(<[^>]+>)?\s*\(' --type ts -C 2Repository: TanStack/table
Length of output: 1967
🏁 Script executed:
# Search for how columnHelper and useTable are used in Vue examples/tests
rg -l 'columnHelper|useTable' packages/vue-tableRepository: TanStack/table
Length of output: 168
🏁 Script executed:
# Check for type tests in the vue-table package
fd '\.test\.|\.spec\.' packages/vue-tableRepository: TanStack/table
Length of output: 40
🏁 Script executed:
# Check the Vue filters example to see if TData is specified elsewhere
cat packages/vue-table/examples/filters/src/tableHelper.tsRepository: TanStack/table
Length of output: 144
🏁 Script executed:
# Compare with Solid variant to see if it has different TData handling
head -40 packages/solid-table/src/createTableHelper.tsRepository: TanStack/table
Length of output: 1168
🏁 Script executed:
# Search for explicit type arguments being used with createTableHelper
rg 'createTableHelper\s*<' --type ts -C 1Repository: TanStack/table
Length of output: 687
🏁 Script executed:
# Check if there are .d.ts or type test files that verify TData inference
find packages/vue-table -name '*.test.*' -o -name '*.spec.*' -o -name '*.d.ts'Repository: TanStack/table
Length of output: 40
🏁 Script executed:
# Check Svelte variant
head -40 packages/svelte-table/src/createTableHelper.tsRepository: TanStack/table
Length of output: 1031
🏁 Script executed:
# Look for TypeScript strict mode or type checking configuration
find packages/vue-table -name 'tsconfig.json' -exec cat {} \;Repository: TanStack/table
Length of output: 369
🏁 Script executed:
# Check if there are any type assertions or workarounds in useTable that might help TData inference
head -100 packages/vue-table/src/useTable.tsRepository: TanStack/table
Length of output: 2935
🏁 Script executed:
# Check TableHelperOptions signature in table-core to understand parameter inference
rg 'type TableHelperOptions' packages/table-core/src -A 10Repository: TanStack/table
Length of output: 928
🏁 Script executed:
# Let's verify the exact difference in signatures between Vue and Svelte
echo "=== Vue createTableHelper signature ===" && \
sed -n '26,31p' packages/vue-table/src/createTableHelper.ts && \
echo && echo "=== Svelte createTableHelper signature ===" && \
sed -n '26,31p' packages/svelte-table/src/createTableHelper.ts && \
echo && echo "=== Solid createTableHelper signature ===" && \
sed -n '23,28p' packages/solid-table/src/createTableHelper.tsRepository: TanStack/table
Length of output: 782
🏁 Script executed:
# Check if Vue's useTable can infer TData from tableOptions parameter
rg 'useTable\s*<' packages/vue-table --type ts -B 2 -A 5Repository: TanStack/table
Length of output: 1141
Vue createTableHelper lacks a default for TData, preventing type inference.
The function signature has TData extends RowData without a default value (unlike Solid/Svelte which use = any), and the tableHelperOptions parameter doesn't reference TData. This means callers must provide explicit type arguments—createTableHelper<Features, DataType>({...})—or TData falls back to the constraint type. Compare with Svelte/Solid which mitigate this with a default, allowing lenient inference when the type isn't supplied.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vue-table/src/createTableHelper.ts` at line 15, The generic TData on
createTableHelper currently lacks a default which prevents inference; change the
declaration from "TData extends RowData" to "TData extends RowData = any" and
make the tableHelperOptions parameter type reference TData (so the options
signature ties to the TData generic) so callers don't need to supply explicit
type args; update the related type use of TableHelper_Core<TFeatures> & { ... }
to preserve existing members (including tableCreator) while allowing TData to
default and be inferred via tableHelperOptions.
| watchEffect(() => { | ||
| allState.value | ||
| allOptions.value | ||
| notifier.value++ | ||
| }) |
There was a problem hiding this comment.
Infinite loop: watchEffect creates self-dependency on notifier.
The expression notifier.value++ reads notifier.value before incrementing, creating a reactive dependency on notifier itself. When the increment writes the new value, the effect re-triggers, causing an infinite loop.
🐛 Proposed fix: use `watch` to avoid self-dependency
- watchEffect(() => {
- allState.value
- allOptions.value
- notifier.value++
- })
+ watch(
+ [allState, allOptions],
+ () => {
+ notifier.value++
+ },
+ { immediate: true },
+ )Using watch with explicit sources avoids creating a dependency on notifier, so the increment only occurs when allState or allOptions change.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| watchEffect(() => { | |
| allState.value | |
| allOptions.value | |
| notifier.value++ | |
| }) | |
| watch( | |
| [allState, allOptions], | |
| () => { | |
| notifier.value++ | |
| }, | |
| { immediate: true }, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vue-table/src/useTable.ts` around lines 123 - 127, The current
watchEffect creates a self-dependency because notifier.value is read before
being incremented; replace the watchEffect with a watch that explicitly watches
[allState, allOptions] as sources and increments notifier.value inside the
callback (optionally with { immediate: true } if the initial run is required).
Update the reactive watcher around watchEffect -> watch so the effect does not
depend on notifier and only runs when allState or allOptions change.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/solid-table/src/createTable.ts (1)
73-83: Minor type inconsistency inmergeOptionssignature.The
optionsparameter at line 77 is typed asTableOptions<TFeatures, TData>, but the core type definition expectsPartial<TableOptions<TFeatures, TData>>. This won't cause runtime issues but creates a type mismatch with the interface contract.🔧 Suggested fix
mergeOptions: ( defaultOptions: TableOptions<TFeatures, TData>, - options: TableOptions<TFeatures, TData>, + options: Partial<TableOptions<TFeatures, TData>>, ) => { return mergeProps(defaultOptions, options) },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/solid-table/src/createTable.ts` around lines 73 - 83, The mergeOptions callback in createTable (the inline function assigned to resolvedOptions.mergeOptions) has its second parameter typed as TableOptions<TFeatures, TData> but should be Partial<TableOptions<TFeatures, TData>> to match the expected interface; update the signature of the options parameter in that mergeOptions function to Partial<TableOptions<TFeatures, TData>> (keeping the return as mergeProps(defaultOptions, options)) so the types align with the core type definition.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/vue/row-selection/src/App.vue`:
- Around line 107-110: The getter enableRowSelection currently shadows the
reactive ref name and may not clearly reference the reactive ref used by Vue;
change the getter to return the actual ref's value (e.g., return
enableRowSelectionRef.value or whatever the ref is named in the module) so it
doesn't shadow the variable, and ensure the code that creates cell handlers
(row.getToggleSelectedHandler()) is called inside the reactive render context
(e.g., within the component render or a watchEffect in useTable) so Vue will
re-run the cell render and recreate the handler when the ref changes.
In `@packages/angular-table/src/injectTable.ts`:
- Around line 196-201: The computed assigned to the 'value' property on table
returns the same table reference, so Angular signal referential equality
prevents updates; update the Object.defineProperty block that sets table.value
(the computed(() => { stateNotifier(); return table })) to either supply a
custom equality option (e.g., computed(..., { equal: () => false })) or return
an immutable wrapper object (e.g., computed(() => ({ table, _tick:
stateNotifier() }))) so consumers see a changed value when stateNotifier()
fires; locate the computed call in the Object.defineProperty for table.value and
implement one of these fixes.
---
Nitpick comments:
In `@packages/solid-table/src/createTable.ts`:
- Around line 73-83: The mergeOptions callback in createTable (the inline
function assigned to resolvedOptions.mergeOptions) has its second parameter
typed as TableOptions<TFeatures, TData> but should be
Partial<TableOptions<TFeatures, TData>> to match the expected interface; update
the signature of the options parameter in that mergeOptions function to
Partial<TableOptions<TFeatures, TData>> (keeping the return as
mergeProps(defaultOptions, options)) so the types align with the core type
definition.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1246ecba-e9b1-45a7-92fe-52ee713e26a0
📒 Files selected for processing (4)
examples/vue/row-selection/src/App.vuepackages/angular-table/src/injectTable.tspackages/angular-table/tests/angularReactivityFeature.test.tspackages/solid-table/src/createTable.ts
| Object.defineProperty(table, 'value', { | ||
| value: computed(() => { | ||
| stateNotifier() | ||
| return table | ||
| }), | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check how other adapters (Solid, Vue) implement similar value signals
# Search for value signal patterns in other adapters
rg -n "value.*computed|value.*signal" --type ts -g '*table*' -g '!*test*' -A 3 -B 1Repository: TanStack/table
Length of output: 1336
🌐 Web query:
Angular signals computed equality behavior same reference
💡 Result:
Angular signals (including computed) use an equality check to decide whether a “new” value should be considered changed and therefore notify dependents.
- Default behavior: the default equality function is effectively
Object.is. For objects/arrays, that means referential equality—if you return the same object/array reference, it’s considered unchanged, so dependents won’t re-render/react (even if you mutated properties inside that object). [1] - For
computed: when its dependencies change, Angular may re-run the computation, but it will only publish/propagate the result if the computed’s new value is not equal to the previous value under its equality function. You can customize this viacomputed(..., { equal }). [1][2] - How to handle “same reference” cases:
- Prefer immutable updates (return a new object/array reference), or
- Provide a custom equality function (e.g., deep compare, or
() => falseto always notify). [1][2]
Sources: Signals guide (equality functions) [1], CreateComputedOptions API (equal) [2].
Use a custom equality function or immutable wrapper for the table.value signal.
The computed returns the same table reference on every dependency change. Angular signals use referential equality (Object.is) by default, so returning the same object reference is treated as "unchanged"—consumers won't be notified of updates even when stateNotifier() fires.
To fix this, either:
- Provide a custom equality function that always returns
false:computed(() => { stateNotifier(); return table; }, { equal: () => false }) - Wrap in an immutable container:
computed(() => ({ table: stateNotifier(), value: table }))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/angular-table/src/injectTable.ts` around lines 196 - 201, The
computed assigned to the 'value' property on table returns the same table
reference, so Angular signal referential equality prevents updates; update the
Object.defineProperty block that sets table.value (the computed(() => {
stateNotifier(); return table })) to either supply a custom equality option
(e.g., computed(..., { equal: () => false })) or return an immutable wrapper
object (e.g., computed(() => ({ table, _tick: stateNotifier() }))) so consumers
see a changed value when stateNotifier() fires; locate the computed call in the
Object.defineProperty for table.value and implement one of these fixes.

Table will expose a
optionsStorewhich is a store storing the table options. This remove the necessity to calltable.baseStore.setOptions(prev => ({...prev}))on everysetOptionsin our adapters.Since
optionswill be reactive and patched with thenotifiergiven by the adapter through thetableReactivityFeature, solid/vue table will update even if an option change after table construction (properties given as a getter)Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores